Skip to content

ARROW-9701: [CI][Java] Add a job for s390x Java on TravisCI#7938

Closed
kiszk wants to merge 12 commits into
apache:masterfrom
kiszk:ARROW-9701
Closed

ARROW-9701: [CI][Java] Add a job for s390x Java on TravisCI#7938
kiszk wants to merge 12 commits into
apache:masterfrom
kiszk:ARROW-9701

Conversation

@kiszk

@kiszk kiszk commented Aug 12, 2020

Copy link
Copy Markdown
Member

This PR adds a new test job for big-endian Java environment to TravisCI. It still keeps as

   allow_failures:
     - arch: s390x

@github-actions

Copy link
Copy Markdown

Comment thread .travis.yml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needless because we don't use Ubuntu based docker image.

Comment thread .travis.yml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needless because ARCH is already passed as environment variable.

Comment thread .travis.yml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using a common script for all jobs like the following?

diff --git a/.travis.yml b/.travis.yml
index fa5d84c82..551d0a029 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -47,15 +47,23 @@ jobs:
       env:
         ARCH: s390x
         ARROW_CI_MODULES: "CPP"
-        ARROW_FLIGHT: "ON"
-        ARROW_PARQUET: "OFF"
         DOCKER_IMAGE_ID: ubuntu-cpp
-        PARQUET_BUILD_EXAMPLES: "OFF"
-        PARQUET_BUILD_EXECUTABLES: "OFF"
-        Protobuf_SOURCE: "BUNDLED"
+        DOCKER_RUN_ARGS: >-
+          -e ARROW_FLIGHT=ON
+          -e ARROW_PARQUET=OFF
+          -e PARQUET_BUILD_EXAMPLES=OFF
+          -e PARQUET_BUILD_EXECUTABLES=OFF
+          -e Protobuf_SOURCE=BUNDLED
+          -e cares_SOURCE=BUNDLED
+          -e gRPC_SOURCE=BUNDLED
         UBUNTU: "20.04"
-        cares_SOURCE: "BUNDLED"
-        gRPC_SOURCE: "BUNDLED"
+    - name: "Java on s390x"
+      os: linux
+      arch: s390x
+      env:
+        ARCH: s390x
+        ARROW_CI_MODULES: "JAVA"
+        DOCKER_IMAGE_ID: debian-java
   allow_failures:
     - arch: s390x
 
@@ -89,13 +97,7 @@ script:
     ulimit -c unlimited || :
   - |
     archery docker run \
-      -e ARROW_FLIGHT=${ARROW_FLIGHT:-OFF} \
-      -e ARROW_PARQUET=${ARROW_PARQUET:-ON} \
-      -e PARQUET_BUILD_EXAMPLES=${PARQUET_BUILD_EXAMPLES:-ON} \
-      -e PARQUET_BUILD_EXECUTABLES=${PARQUET_BUILD_EXECUTABLES:-ON} \
-      -e Protobuf_SOURCE=${Protobuf_SOURCE:-} \
-      -e cares_SOURCE=${cares_SOURCE:-} \
-      -e gRPC_SOURCE=${gRPC_SOURCE:-} \
+      ${DOCKER_RUN_ARGS} \
       --volume ${PWD}/build:/build \
       ${DOCKER_IMAGE_ID}

Comment thread ci/scripts/java_build.sh Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
grp="com.github.icexelloss"
group="com.github.icexelloss"

Comment thread ci/scripts/java_build.sh Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cls="linux-s390_64"
classifier="linux-s390_64"

Comment thread ci/scripts/java_build.sh Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, $(...) is preferred than `...`:

Suggested change
${mvn_install} -DgroupId=${grp} -DartifactId=${artifact} -Dversion=${ver} -Dpackaging=${extension} -Dfile=`pwd`/${target}
${mvn_install} -DgroupId=${grp} -DartifactId=${artifact} -Dversion=${ver} -Dpackaging=${extension} -Dfile=$(pwd)/${target}

@kiszk kiszk marked this pull request as draft August 15, 2020 12:49
@kiszk kiszk force-pushed the ARROW-9701 branch 2 times, most recently from ffbb910 to 6fd6dac Compare August 27, 2020 02:43
@kiszk kiszk marked this pull request as ready for review August 28, 2020 00:31
@kiszk

kiszk commented Aug 28, 2020

Copy link
Copy Markdown
Member Author

@kou Thank you. Now, a TravisCI job is finished correctly with many failures. Could you review this again?

Comment thread ci/scripts/java_build.sh Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove -Dmaven.compiler.verbose=true and remove the else clause?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, done

Comment thread java/pom.xml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, use the original setting. Done

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kiszk

kiszk commented Oct 3, 2020

Copy link
Copy Markdown
Member Author

@emkornfield @kou Is it ok to merge this PR now? Or, do we still hold off for a while?

@emkornfield

Copy link
Copy Markdown
Contributor

Yes, I think so. I'd still like to update the contributor guidelines to make it clear what the level of support we are aiming for in each language (I hope to get to it tonight but if I don't it might take me a few days), but I think CI should be harmless. @kou it looks like you are happy with this PR?

@kou

kou commented Oct 5, 2020

Copy link
Copy Markdown
Member

Yes.
But I want to confirm that this still works with the latest master before we merge.
@kiszk Could you rebase this on the latest master?

@kiszk

kiszk commented Oct 5, 2020

Copy link
Copy Markdown
Member Author

@emkornfield @kou Thank you. I want to merge only this PR for CI. We should still hold off other PRs.

I rebased with master branch and confirmed it works well.

@kou

kou commented Oct 5, 2020

Copy link
Copy Markdown
Member

Thanks.
I'll merge this.

@kou kou closed this in 8c75941 Oct 5, 2020
kou pushed a commit that referenced this pull request Oct 6, 2020
This is a follow-up of #7938. #7938 forgot downloading a required dll `libprotobuf.so.18` at the build time.

Closes #8368 from kiszk/ARROW-10200

Authored-by: Kazuaki Ishizaki <ishizaki@jp.ibm.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants